Skip to content
This repository has been archived by the owner on Nov 20, 2024. It is now read-only.

Add missed case for prefer_collection_literals #3705

Merged
merged 3 commits into from
Sep 21, 2022

Conversation

natebosch
Copy link
Member

Shows that the check for returning from a function literal is incorrect.

Description

Please include a summary of the change and a reference to any corresponding issues.
If this is a fix to an existing lint rule or a proposed new one, please include
the name of the rule in the pull request.

If this is a new lint or feature and there is not an existing tracking bug, please
consider opening one to better facilitate conversation.

Fixes # (issue)

Shows that the check for returning from a function literal is incorrect.
@coveralls
Copy link

coveralls commented Sep 20, 2022

Coverage Status

Coverage remained the same at 95.761% when pulling 24e499d on repro-collection-literal-false-negative-closure into a9f509a on main.

@srawlins
Copy link
Member

I think someone is working on a "FAILED LINT" syntax for the linter test cases.

Copy link
Member

@bwilkerson bwilkerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to comment this line so that it's clear that the failure is incorrect.

Even better would be to write this as a reflective test so that it could be marked as a @failingTest with the correct expectations. (See https://github.com/dart-lang/linter/blob/main/test/rules/prefer_collection_literals_test.dart.)

Other than that, lgtm.

@bwilkerson
Copy link
Member

I think someone is working on a "FAILED LINT" syntax for the linter test cases.

I hope not. We don't need it given that we've decided to switch over to using test_reflective_loader.

@srawlins
Copy link
Member

I hope not. We don't need it given that we've decided to switch over to using test_reflective_loader.

Ah that's good, this PR can be converted to that, then?

@bwilkerson
Copy link
Member

I wouldn't block it from being merged because it hadn't been converted, but that was my suggestion.

@srawlins
Copy link
Member

I wouldn't block it from being merged because it hadn't been converted, but that was my suggestion.

But the introduced test is failing. How could we merge it?

@bwilkerson
Copy link
Member

But the introduced test is failing. How could we merge it?

I didn't notice that. (I mean, who pays attention to the bots? :-) ) Then yes, it needs to be changed, so making it a reflective test makes the most sense.

Test is currently failing because the lint has a bug checking for
correct returns within function literals.
@natebosch natebosch marked this pull request as ready for review September 21, 2022 18:32
@natebosch
Copy link
Member Author

I'm pretty sure I got the offset correct...

Copy link
Member

@bwilkerson bwilkerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that the offset is wrong, but it still fails, so it's probably close enough. We have some support coming down the line that will make tests like this much easier to write.

@natebosch natebosch merged commit ff18297 into main Sep 21, 2022
@natebosch natebosch deleted the repro-collection-literal-false-negative-closure branch September 21, 2022 19:41
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 23, 2023
Test is currently failing because the lint has a bug checking for
correct returns within function literals.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants